-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tidy check to ensure rustdoc ui tests parity #87845
Add tidy check to ensure rustdoc ui tests parity #87845
Conversation
This comment has been minimized.
This comment has been minimized.
…oc` are present in `src/test/rustdoc-ui`
8c1d614
to
9a4c5fb
Compare
Hm, this seems like an odd fix - if we expect that one of these directories is a subset of the other, I would expect us to not have two directories but instead annotate files with the compiletest revision support or a new dedicated feature to this effect. The current check just checks for same file name in both directories, which seems quite error prone - either file can get edited easily. I think either we should move to revisions or a similar style feature. But I'm not sure the premise of this is quite right. If the code to validate attributes is in rustc's attribute validation, then rustdoc simply must run that - I don't think in that case we need to duplicate this for all tests. We don't rerun regular UI tests with rustdoc to make sure it's not using a different rustc parser or something; this seems like a similar case. |
Yes, it's not great as is. For context: for a long time, rustdoc hid most attributes checks, preventing the warnings from rustc to be deniable in rustdoc. The idea was then to ensure this behaviour doesn't happen again. |
This can happen anyway though: because of #73566, the lints aren't ever emitted in the first place. This misleads people working on rustdoc because they could think it's a bug on their end that the lint shows up in rustc and not rustdoc, when in fact that's intentional. The tests only pass now because we happen to only test lints that run in rustdoc. |
I think it makes sense for T-rustdoc to decide whether/how this check should look. |
Like I said above, I don't think we should add this check. |
Closing then. |
In #87728, I discovered that it wasn't known that tests present in
src/test/ui/rustdoc
should also be present insrc/test/rustdoc-ui
(because we need to ensure that the rustdoc attributes and errors are thrown by both rustc and rustdoc).cc @jyn514 @camelid
r? @Mark-Simulacrum